Skip to content

Add CentOS CoreOS (SCOS) & Red Hat Linux CoreOS (RHCOS)#1674

Open
CrAazZyMaN21 wants to merge 1 commit intonetbootxyz:developmentfrom
CrAazZyMaN21:development
Open

Add CentOS CoreOS (SCOS) & Red Hat Linux CoreOS (RHCOS)#1674
CrAazZyMaN21 wants to merge 1 commit intonetbootxyz:developmentfrom
CrAazZyMaN21:development

Conversation

@CrAazZyMaN21
Copy link

Solves #1553

Also adding CentOS CoreOS for the OKD project 😃

@CrAazZyMaN21 CrAazZyMaN21 force-pushed the development branch 2 times, most recently from 9985e79 to 11ef6a7 Compare September 12, 2025 20:45
@CrAazZyMaN21 CrAazZyMaN21 marked this pull request as draft September 12, 2025 20:52
@CrAazZyMaN21 CrAazZyMaN21 force-pushed the development branch 2 times, most recently from 8ff37bc to 9bac5d2 Compare September 12, 2025 20:59
@CrAazZyMaN21 CrAazZyMaN21 marked this pull request as ready for review September 12, 2025 21:05
@antonym
Copy link
Member

antonym commented Sep 12, 2025

@claude review

@claude
Copy link
Contributor

claude bot commented Sep 12, 2025

Claude finished @antonym's task —— View job


Code Review: Add CentOS CoreOS & Red Hat Linux CoreOS

I've reviewed PR #1674 which adds support for CentOS CoreOS and Red Hat Linux CoreOS. Here's my comprehensive analysis:

Positive Aspects

  1. Addresses Valid Need: The PR solves issue [Request] RedHat CoreOS (RHCOS) for OpenShift PXE Boot #1553 and adds support for two important CoreOS variants used in enterprise environments
  2. Follows Existing Patterns: The implementation appears to follow the established pattern used by Fedora CoreOS
  3. Appropriate File Structure: New template files follow the naming convention (centos-coreos.ipxe.j2, rhcos.ipxe.j2)

⚠️ Areas for Review & Potential Issues

1. Configuration Structure Concerns
Based on the existing CoreOS configuration in roles/netbootxyz/defaults/main.yml:191-203, I notice:

  • The Fedora CoreOS configuration has specific base_dir, mirror, and versioning structure
  • Need to ensure CentOS CoreOS and RHCOS have appropriate configuration entries with:
    • Proper mirror URLs and base directories
    • Correct versioning scheme
    • Appropriate naming conventions

2. Template File Consistency
From examining the existing coreos.ipxe.j2 template, the new templates should:

  • Follow the same parameter structure (install_device, ignition_url)
  • Use consistent variable naming and menu structure
  • Handle architecture detection properly (x86_64 vs aarch64)
  • Include proper error handling and exit paths

3. README Integration
The README.md should be updated to include:

  • CentOS CoreOS entry in the Operating Systems table (around line 138 where Fedora CoreOS is listed)
  • Red Hat CoreOS entry with appropriate URL references
  • Consistent formatting with existing entries

4. Missing Configuration Verification
Since I cannot access the actual PR files due to branch access limitations, I recommend verifying:

For main.yml additions:

releases:
  centos-coreos:
    base_dir: [appropriate path]
    enabled: true
    menu: linux
    mirror: [official CentOS CoreOS mirror]
    name: CentOS CoreOS
    versions:
    - code_name: [version]
      name: [display name]
  
  rhcos:
    base_dir: [appropriate path] 
    enabled: true
    menu: linux
    mirror: [official RHCOS mirror]
    name: Red Hat CoreOS
    versions:
    - code_name: [version]
      name: [display name]

For template files:

  • Ensure URL patterns match official distribution methods
  • Verify kernel/initrd/rootfs URL structures
  • Test ignition config integration
  • Validate install device handling

🔍 Recommendations

  1. Verify Mirror URLs: Ensure the mirror URLs point to official, stable sources
  2. Test Boot Process: Validate that the boot process works with real hardware/VMs
  3. Documentation: Consider adding usage examples or links to official documentation
  4. Versioning Strategy: Ensure the versioning scheme aligns with upstream release patterns

📝 Final Assessment

The PR appears to be a solid addition that follows established patterns. However, without access to the actual implementation details, I recommend:

  1. Manual testing of the boot sequences
  2. Verification of mirror URL stability and availability
  3. Ensuring the configuration aligns with official CoreOS installation methods

Once these items are confirmed, this should be a valuable addition to netboot.xyz's distribution support.


@CrAazZyMaN21
Copy link
Author

Hi, I tried to find the issue why the Build PR Pipeline failed but I can't seem to find it. I am guessing the file name "centos-coreos.ipxe.j2" is not suitable and should be changed to e.g "scos.ipxe.j2"

@CrAazZyMaN21 CrAazZyMaN21 changed the title Add CentOS CoreOS & Red Hat Linux CoreOS Add CentOS CoreOS (SCOS) & Red Hat Linux CoreOS (RHCOS) Sep 15, 2025
@CrAazZyMaN21
Copy link
Author

@antonym Can you please let the workflow run again. I think it is fixed now and I squashed the commits.

@CrAazZyMaN21 CrAazZyMaN21 force-pushed the development branch 3 times, most recently from 2130856 to 4da58ab Compare November 9, 2025 14:08
@CrAazZyMaN21
Copy link
Author

  • Added RHCOS 4.20
  • Moved RHCOS 4.18 to Maintenance Support

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds support for two CoreOS variants to netboot.xyz: CentOS Stream CoreOS (SCOS) for the OKD project and Red Hat Enterprise Linux CoreOS (RHCOS) for OpenShift deployments. Both operating systems are designed for container-focused workloads and use the CoreOS installer framework with Ignition configuration.

Key Changes:

  • Added iPXE boot menu templates for SCOS and RHCOS with support for interactive installation device and Ignition config URL configuration
  • Added release configurations with version definitions and mirror URLs for both operating systems
  • Updated README documentation to include both new OS entries in the supported operating systems table

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
roles/netbootxyz/templates/menu/scos.ipxe.j2 New iPXE menu template for CentOS Stream CoreOS with multi-version support and configurable installation parameters
roles/netbootxyz/templates/menu/rhcos.ipxe.j2 New iPXE menu template for Red Hat Enterprise Linux CoreOS with OpenShift version support
roles/netbootxyz/defaults/main.yml Added configuration entries for both RHCOS (versions 4.12-4.20) and SCOS (versions 9.x and 10.x) with mirror URLs and version metadata
README.md Added documentation entries for both CoreOS variants in the operating systems compatibility table

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


:install_dev
echo -n Selected install device will be overwritten,
echo -n please ensure you know what are you doing...
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammar issue: "ensure you know what are you doing" should be "ensure you know what you are doing" (remove "are" before "you").

Suggested change
echo -n please ensure you know what are you doing...
echo -n please ensure you know what you are doing...

Copilot uses AI. Check for mistakes.

:install_dev
echo -n Selected install device will be overwritten,
echo -n please ensure you know what are you doing...
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammar issue: "ensure you know what are you doing" should be "ensure you know what you are doing" (remove "are" before "you").

Suggested change
echo -n please ensure you know what are you doing...
echo -n please ensure you know what you are doing...

Copilot uses AI. Check for mistakes.
{% for item in releases.rhcos.versions %}
iseq ${core_version} {{ item.code_name }} ||
{% endfor %}
set base_url ${rhcos_mirror}/${os_arch}/dependencies/rhcos
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The base_url construction doesn't use the rhcos_base_dir variable that is defined in the configuration (set to "pub/openshift-v4"). The URL path should likely include this base directory. Based on the configuration, consider whether the URL should be ${rhcos_mirror}/${rhcos_base_dir}/${os_arch}/dependencies/rhcos instead of ${rhcos_mirror}/${os_arch}/dependencies/rhcos, or if the base_dir configuration needs to be adjusted to match the actual URL structure.

Suggested change
set base_url ${rhcos_mirror}/${os_arch}/dependencies/rhcos
set base_url ${rhcos_mirror}/${rhcos_base_dir}/${os_arch}/dependencies/rhcos

Copilot uses AI. Check for mistakes.

:rhcos_boot
{% for item in releases.rhcos.versions %}
iseq ${core_version} {{ item.code_name }} ||
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable name is incorrect. This line uses ${core_version} but should use ${rhcos_version} to match the variable set on line 23 and used throughout the rest of the file. This will cause the version comparison to fail since core_version is never set in this template.

Suggested change
iseq ${core_version} {{ item.code_name }} ||
iseq ${rhcos_version} {{ item.code_name }} ||

Copilot uses AI. Check for mistakes.
Comment on lines 31 to 33
{% for item in releases.rhcos.versions %}
iseq ${core_version} {{ item.code_name }} ||
{% endfor %}
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop appears incomplete. It checks the version but doesn't set any variable or perform any action. Comparing with the scos.ipxe.j2 template (line 32), this should set a variable like set rhcos_major_version {{ item.major_version }} or similar. However, RHCOS versions in the configuration don't have a major_version field, so the logic may need to be different or the loop may not be needed at all.

Suggested change
{% for item in releases.rhcos.versions %}
iseq ${core_version} {{ item.code_name }} ||
{% endfor %}

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +453 to +465
name: 4.12 [Extended Support]
- code_name: 4.14
name: 4.14 [Extended Support]
- code_name: 4.16
name: 4.16 [Maintenance Support]
- code_name: 4.17
name: 4.17 [Maintenance Support]
- code_name: 4.18
name: 4.18 [Maintenance Support]
- code_name: 4.19
name: 4.19 [Full Support]
- code_name: 4.20
name: 4.20 [Full Support]
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The RHCOS version entries are missing the major_version field that is present in the SCOS configuration. While the RHCOS template currently doesn't use major_version in its URL construction (unlike SCOS), the incomplete loop at lines 31-33 in rhcos.ipxe.j2 suggests this field might have been intended. Either add the major_version field to each version entry if it's needed for future URL construction, or remove the useless loop in the template file.

Suggested change
name: 4.12 [Extended Support]
- code_name: 4.14
name: 4.14 [Extended Support]
- code_name: 4.16
name: 4.16 [Maintenance Support]
- code_name: 4.17
name: 4.17 [Maintenance Support]
- code_name: 4.18
name: 4.18 [Maintenance Support]
- code_name: 4.19
name: 4.19 [Full Support]
- code_name: 4.20
name: 4.20 [Full Support]
name: 4.12 [Extended Support]
major_version: 4
- code_name: 4.14
name: 4.14 [Extended Support]
major_version: 4
- code_name: 4.16
name: 4.16 [Maintenance Support]
major_version: 4
- code_name: 4.17
name: 4.17 [Maintenance Support]
major_version: 4
- code_name: 4.18
name: 4.18 [Maintenance Support]
major_version: 4
- code_name: 4.19
name: 4.19 [Full Support]
major_version: 4
- code_name: 4.20
name: 4.20 [Full Support]
major_version: 4

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants